Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

targets: aarch64: Add bare-metal aarch64 target #53233

Merged
merged 1 commit into from
Aug 14, 2018
Merged

targets: aarch64: Add bare-metal aarch64 target #53233

merged 1 commit into from
Aug 14, 2018

Conversation

andre-richter
Copy link
Member

A generic AArch64 target that can be used for writing bare-metal code
for 64-bit ARM architectures.

A generic AArch64 target that can be used for writing bare-metal code
for 64-bit ARM architectures.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @varkor (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2018
@varkor
Copy link
Member

varkor commented Aug 9, 2018

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned varkor Aug 9, 2018
@alexcrichton
Copy link
Member

cc @japaric, does the naming here look ok to you?

Otherwise all the technical changes here look good to me!

@japaric
Copy link
Member

japaric commented Aug 10, 2018

@andre-richter does aarch64 also have two float ABIs: soft and hard? Or do all ARMv8 processors have a hardware FPU? If all processors are using the hard float ABI then the name seems fine; otherwise we should include the fourth field (abi) to be able to differentiate soft float from hard float.

@andre-richter
Copy link
Member Author

andre-richter commented Aug 10, 2018

@alexcrichton
Copy link
Member

Ok! In that case let's...

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 10, 2018

📌 Commit 898950c has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2018
@parched
Copy link
Contributor

parched commented Aug 10, 2018

Could be worth putting something in the ABI part of the triple. armclang uses eabi, Linaro uses elf. I'm also curious why this target uses LLD but the other bare-metal ones use GCC?

@parched
Copy link
Contributor

parched commented Aug 10, 2018

Could be worth putting something in the ABI part of the triple.

Although, I think whatever you put in there is ignored by LLVM for AArch64 anyway. So nothing is should be fine but then again the other rust targets where ABI doesn't matter have "elf" but that's the object format not ABI so I don't know.

@andre-richter
Copy link
Member Author

andre-richter commented Aug 10, 2018

I spent some thoughts on your questions as well before I started the PR.

Regarding the object-format, I came to the same conclusion as you that it doesn't matter, because for bare-metal we pipe it through objcopy afterwards anyways. So we don't care what we get as intermediary result.
Additionally, the llvm Target we use is named aarch64-unknown-none as well, so we are consistent there.

I used lld because it worked fine for me, and is the more native and self-contained solution within the rust toolchain in my opinion.

@parched
Copy link
Contributor

parched commented Aug 10, 2018

@japaric

does aarch64 also have two float ABIs: soft and hard? Or do all ARMv8 processors have a hardware FPU? If all processors are using the hard float ABI then the name seems fine; otherwise we should include the fourth field (abi) to be able to differentiate soft float from hard float.

I would've thought most people will use this target for kernel development where they want to disable the FPU to avoid saving the FP registers on every context switch/syscall so perhaps add target feature -fp-armv8?

@andre-richter
Copy link
Member Author

Would it matter if you don't use float types/routines? And saving float regs Is not happening unless you do it by hand? Am I missing something?

Could be added for safety, though, yes.

@parched
Copy link
Contributor

parched commented Aug 10, 2018

Would it matter if you don't use float types/routines?

Yes because the compiler can auto vectorize which will use fp/simd registers.

And saving float regs Is not happening unless you do it by hand?

Yes.

This target is valid as it is I'm just not sure which is a more useful target, one with FP or one without. Maybe add both ;). I guess most kernel devs will create their own target spec to suit, though.

@andre-richter
Copy link
Member Author

Thanks for the vectorization example, certainly haven't thought about that.

Like described in the source code comments, the usage intent is anyways to use something like .cargo/config to further specialize by e.g. defining a specific cpu. Disabling fpu explicitly could be done in the same take ideally if wished.

@parched
Copy link
Contributor

parched commented Aug 10, 2018

Disabling fpu explicitly could be done in the same take ideally if wished.

Unfortunately not because libcore won't be recompiled so the FPU usage would still be there plus you end up with an ABI mismatch between libcore and the rest of your code.

@andre-richter
Copy link
Member Author

andre-richter commented Aug 10, 2018

Oh, will adding a target like that automatically ship with a precompiled core? No additional hooks needed? I thought we need xargo anyways to use it.

Not using fp is definitely the more likely default case for this target. I will submit another patch to fix that. Too late to opt out now I guess.

@japaric
Copy link
Member

japaric commented Aug 10, 2018

@parched

I'm also curious why this target uses LLD but the other bare-metal ones use GCC?

Presumably because LLD has good enough AArch64 support and rust-lld exists today. The ARM Cortex-M targets were added when rust-lld didn't exist and LLD didn't support ARM and linker scripts well enough to build bare metal ARM binaries. We actually want to change the default Cortex-M to rust-lld (see rust-embedded/wg#160) before it's too late.

So nothing is should be fine but then again the other rust targets where ABI doesn't matter have "elf" but that's the object format not ABI so I don't know.

Some targets are named after the prefix of the canonical GCC toolchain (e.g. msp430-none-elf); others use the llvm triple. The fourth field of the llvm triple can be one of: environment, abi or object format. msp430-none-elf (equivalent to msp430-unknown-none-elf) is also a valid llvm triple.

@andre-richter

Oh, will adding a target like that automatically ship with a precompiled core?

No, this PR won't enable builds of rust-std. At the very least you'll have to add the target name to this file.

Not using fp is definitely the more likely default case for this target.

If there's a use case for forbidding the use of the FPU even though all processors have an FPU I would suggest adding two targets: aarch64-unknown-none-eabi{,hf}. It doesn't seem unlikely that, in the future, someone will want to write a bare metal application that does use the FPU (probably not a kernel) -- the second target would be for them.

@andre-richter
Copy link
Member Author

andre-richter commented Aug 11, 2018

Thanks @japaric for clearing stuff up.

Let me reiterate what I wanted to achieve with this target.
It should be a starting point for people writing bare-metal for aarch64. By starting point, like mentioned before, I mean that people ideally inject additional flags to make their bare-metal software more specific to the HW they will write it for (e.g. target-cpu).

Oh, will adding a target like that automatically ship with a precompiled core?

No, this PR won't enable builds of rust-std. At the very least you'll have to add the target name to this file.

This is good news to me because not getting core/std libraries shipped was my intention. If you write target-optimized (e.g. target-cpu) bare-metal, in my opinion it makes sense to also compile core libs with the optimization. Xargo will do that for you and you have to use Xargo with this target as it is.

Now that I know that we need Xargo anyways, disabling FPU can be done by the user of the target without unintended side effects.

Here's two examples of using the target as it is:

  • User1 wants to write a kernel. He will add the following .cargo/config to his project (for example, that's exactly how I would introduce and feature this target in my bare-metal Rust on RaspberryPi3 tutorials once it lands in nightly).
[target.aarch64-unknown-none]
rustflags = [
  "-C", "link-arg=-Tlink.ld",
  "-C", "target-feature=+a53,-fp-armv8",
]
  • User2 wants to write bare-metal with FPU. The same config but without the FPU part can be used.

I don't think it makes sense for us to assume every possible use-case and provide a target for that. There's just too many possibilities. But shipping a canonical starting point to people is fine imo.

For these reasons, I would vote to leave the target exactly as is. You guys fine with that?

@parched
Copy link
Contributor

parched commented Aug 11, 2018

Yeah I'm fine with that.

kennytm added a commit to kennytm/rust that referenced this pull request Aug 11, 2018
targets: aarch64: Add bare-metal aarch64 target

A generic AArch64 target that can be used for writing bare-metal code
for 64-bit ARM architectures.
@japaric
Copy link
Member

japaric commented Aug 11, 2018

Is the plan is to use Xargo here and not make this target available on stable anytime soon then this approach sgtm.

kennytm added a commit to kennytm/rust that referenced this pull request Aug 13, 2018
targets: aarch64: Add bare-metal aarch64 target

A generic AArch64 target that can be used for writing bare-metal code
for 64-bit ARM architectures.
kennytm added a commit to kennytm/rust that referenced this pull request Aug 14, 2018
targets: aarch64: Add bare-metal aarch64 target

A generic AArch64 target that can be used for writing bare-metal code
for 64-bit ARM architectures.
bors added a commit that referenced this pull request Aug 14, 2018
Rollup of 11 pull requests

Successful merges:

 - #53112 (pretty print BTreeSet)
 - #53208 (Don't panic on std::env::vars() when env is null.)
 - #53226 (driver: set the syntax edition in phase 1)
 - #53229 (Make sure rlimit is only ever increased)
 - #53233 (targets: aarch64: Add bare-metal aarch64 target)
 - #53239 (rustc_codegen_llvm: Restore the closure env alloca hack for LLVM 5.)
 - #53246 (A few cleanups)
 - #53257 (Idiomatic improvements to IP method)
 - #53274 (Remove statics field from CodegenCx)
 - #53290 (Make LLVM emit assembly comments with -Z asm-comments)
 - #53317 (Mark prior failure to avoid ICE)
@bors bors merged commit 898950c into rust-lang:master Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants